test(parser): add unit tests for stripWasmSourceMapUrl#37
Conversation
Adds 15 unit tests covering input validation, the same-reference
optimization path, byte-exact reconstruction when stripping, LEB128
multi-byte size decoding, and the spec-allowed case of multiple
sourceMappingURL sections. Brings packages/parser from 0 to 15 tests.
The function is documented in its source as `@internal — exported
for testing only` but previously had no tests.
Tests use small inline byte-array fixtures built from leb128(),
customSection(), and regularSection() helpers — no fixture files,
no I/O, no mocking. All 15 pass via:
pnpm --filter @agentscript/parser test
Also adds packages/parser/test/*.test.ts to the eslint.config.js
allowDefaultProject allowlist, matching the existing entries for
other workspace packages' test directories.
|
Hey @dsorajisto! Thanks for sending this PR! Do we need these tests? They test something very specific, and something which typically isn't used very much - I think it might be better to restate this as a single large test testing an end-to-end workflow rather than 15 tests that might be brittle in the future. |
Hey @awli On brittleness: every assertion is on byte-level input/output, not internal structure. If someone refactored On an end-to-end alternative: an integration test through On cost: all 15 tests run in ~3ms total. That said, happy to adjust if you still feel strongly. Two paths I'd suggest:
Let me know which way you'd like to go. |
2 participants
What
Adds 15 unit tests for
stripWasmSourceMapUrlinpackages/parser/src/wasm-backend.ts, the function that strips thesourceMappingURLcustom section from a WASM binary. Bringspackages/parserfrom 0 to 15 tests.Also adds
packages/parser/test/*.test.tsto theeslint.config.jsallowDefaultProjectallowlist (matching the existing entries for other workspace packages' test directories).Why
The function is documented in its source as:
…but had no tests. It contains the only non-trivial logic in
wasm-backend.tsoutside the dynamic WASM init flow — LEB128 size decoding, section walking, name matching, and byte-exact reconstruction. Several subtle paths (invalid magic, the same-reference optimization when nothing is stripped, multi-byte LEB128 sizes, and section ordering) benefit from explicit unit coverage.How
New test file at
packages/parser/test/wasm-backend.test.tsusing vitest (already configured in the package). Fixtures are built compositionally from small helpers (leb128(),customSection(),regularSection(),bytes()) — no fixture files, no I/O, no mocks.Coverage of the function's branches
totalSize === wasm.lengthoptimization pathtotalSize < wasm.lengthreconstructionnot.toBe(input)+result.buffer !== input.buffer)The same-reference cases lock down the optimization that returns the input unchanged when nothing was stripped — catches the class of bug where someone "optimizes" the reconstruction path to slice the input.
ESLint config change
packages/parser/test/*.test.tswas added toeslint.config.jsallowDefaultProjectto match the existing pattern for other workspace test directories (packages/parser-javascript/test/*.test.ts, etc.). Without this,npx eslintrejects the file with "not found by the project service". One-line change, follows the existing alphabetical-ish ordering.Test Plan
pnpm --filter @agentscript/parser testResult:
pnpm --filter @agentscript/parser test— 15/15 passingnpx eslint --config eslint.config.js packages/parser/test/wasm-backend.test.ts— exit 0pnpm lintat workspace level — passes (12 pre-existing react-refresh warnings inapps/ui/are unrelated to this change)pnpm typecheckverified by CI (locally fails on missingtree-sitterCLI in theparser-tree-sitterbuild cascade — unrelated to this change; see README Quick Start fails on macOS: Node 26 incompatibility and missing tree-sitter CLI prerequisite #33 for that environment gap, addressed by docs(readme): document tree-sitter CLI prereq and recommend mise #36)Checklist